Skip to content

Test/user account#6

Closed
TonyPythoneer wants to merge 40 commits into
mainfrom
test/user-account
Closed

Test/user account#6
TonyPythoneer wants to merge 40 commits into
mainfrom
test/user-account

Conversation

@TonyPythoneer

Copy link
Copy Markdown
Owner

No description provided.

TonyPythoneer and others added 30 commits May 14, 2026 22:56
CLAUDE.md, README, and OBSERVABILITY.md still described the pre-flatten
project structure and Make targets that no longer exist. Fix the
unconditionally-stale facts: real `make` targets, flat repo layout,
boto3 (not django-storages), valid file paths, and the docker-compose
per-process OTEL_SERVICE_NAME wiring in OBSERVABILITY.md Symptom 5.

Code-coupled doc fixes (storage.py rename, apps.py telemetry wiring,
span processor choice) are deferred to their refinement tasks.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…config

- Change setdefault in manage.py/wsgi.py/asgi.py from empty package
  django_thumbnail.settings to django_thumbnail.settings.local so bare
  invocation without DJANGO_SETTINGS_MODULE set no longer crashes
- Remove redundant DJANGO_SETTINGS_TEST prefix from make test; pyproject.toml
  [tool.pytest.ini_options] already pins the test settings module
- Add tasks/refinement/ checklist files tracking the full refinement plan

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Only mark ImageTask as FAILED when retries are exhausted; keep PROCESSING
  during intermediate retries so pollers see accurate state
- Add Outcome.FAIL to distinguish terminal failure from mid-retry in spans
- Remove force_flush() and stale BSP comment — SimpleSpanProcessor exports
  synchronously on span.end(), making the flush a no-op
- Update test to simulate exhausted retries via apply(retries=max_retries)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move login_required_json from utils.py to images/decorators.py; update
  views.py import — auth concern separated from storage concern
- Replace class-attribute config pattern in _S3BaseClient with explicit
  __init__ parameters; InternalS3/PublicS3 pass their own endpoint/config
  at construction — no more declarative-field convention confusion

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nces

- Rename images/utils.py to images/storage.py — name now matches content
- Update imports across models.py, views.py, tasks.py, test_api.py,
  test_smoke.py, and management/commands/test_image_upload.py
- Update patch targets in test_api.py (images.utils.* → images.storage.*)
- Update CLAUDE.md §3 project layout to reflect storage.py and decorators.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erializer

- Add parse_json_body() to decorators.py; replace three inline try/except
  json.loads blocks in images_list, tasks_create, auth_login
- Add functools.wraps to login_required_json so decorated views keep __name__
- Add _gallery_row() private function in views.py (not on the model) to
  serialize Image + task for the HTML gallery; keeps model free of
  presentation-only concerns
- Remove json import and unused ImageStatus import from views.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove blank=True/default="" from thumbnail_key — key is always computed
  at create time via create_with_key(); update 0001_initial migration in-place
- Drop all if thumbnail_key / if original_key None-guards in to_dict(),
  ImageTask.to_dict(), and _gallery_row() — fields are always populated
- Fix latest_task() to use next(iter(self.tasks.all())) so prefetch cache
  is used instead of bypassed by order_by()
- Add prefetch_related("tasks") to images_list GET queryset to eliminate N+1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Delete AWS_S3_ADDRESSING_STYLE, AWS_DEFAULT_ACL, AWS_QUERYSTRING_AUTH from
  base.py — django-storages is not installed; nothing reads these settings
- Extract _fmt_ts() helper to deduplicate created_at formatting in both
  Image.__str__ and ImageTask.__str__; add example output comments
- Remove redundant if-key guard in delete_from_storage() — both keys required

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add tests for `Image.latest_task()` / `current_status()` and
  verify they use the prefetch cache (query count assertion).
- Split S3-error task test: distinguish retry-in-progress
  (Retry raised, status PROCESSING) from retry exhaustion (FAILED).
- Add HTML view tests covering owner isolation — gallery filters
  by user, image delete refuses cross-user requests.
- Smoke test now uses `Image.format_thumbnail_key_from_original()`
  instead of duplicating the key-format logic.

Drop dead `STORAGES` config from settings/test.py (project uses
boto3 directly, not django-storages) and remove the
`test_image_upload` management command — its scenarios are now
covered by the new tests.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace pyright with pyrefly across config and IDE setup:
- pyrefly.toml migrated from pyrightconfig.json
- Drop pyright dev dep
- Zed: pyrefly LSP via .venv/bin/pyrefly + format_on_save via ruff LSP
- Makefile: consolidate lint/format into `make check` (CI-safe) + `make fix`
- Models/views: add type annotations and casts to satisfy pyrefly

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add `[tool.ruff]` to pyproject.toml: target-version py314, line-length 100,
  deliberate lint select (E/W/F/I/UP/B/SIM), exclude .claude
- Apply ruff auto-fixes + format across project (mostly import sorting / wrap)
- Code fixes prompted by new rules:
  * Split long __str__ f-strings in models.py
  * `raise self.retry(...) from exc` in tasks.py (B904)
  * Narrow bare Exception → ConnectionError in test_tasks.py (B017)
- CI workflows: both use `uv sync --group dev` + setup-uv cache;
  ci-app-integration-test runs `make check` + `make test` instead of inline
- Makefile: add `make migrate` target
- Update tasks/refinement/09 checklist + acceptance criteria

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thumbnail() decodes the file already, so the prior verify() pass was
redundant work on the same buffer. Collapse _validate_image_buffer +
_create_thumbnail_buffer into one _make_thumbnail() that catches
UnidentifiedImageError/OSError as InvalidImageError.

Closes the last open box in tasks/refinement/06_tasks_cleanup.md.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Symptom 1/3 claimed OTel was wired in apps.py:ready(); actual init
lives in manage.py via setup_otel_for_django_web(). Rewrote samples
to match. Dropped non-existent setup_telemetry() call. Reframed
BatchSpanProcessor note as production consideration (task 06 kept
SimpleSpanProcessor only).

Task 01 acceptance: all doc links resolve; README Quick Start
statically verified against Makefile + docker-compose; runtime
clone-follow deferred to task 10.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Telemetry init no longer rides manage.py:
- New django_thumbnail/apps.py with CoreConfig.ready() — gated on
  _is_web_server_process() (runserver / gunicorn / uvicorn) so it
  doesn't fire in the Celery worker parent (gRPC channel would
  inherit across fork) or in management commands.
- INSTALLED_APPS gets CoreConfig as the first entry so OTel init
  precedes other apps' ready().
- manage.py reverts to stock Django boilerplate.
- images/apps.py stays a pure domain app config — no instrumentation
  responsibility leaks into domain apps.

Worker keeps its per-child worker_process_init handler (the celery-
blessed fork-safe hook). Web vs worker init now mirror their
framework-blessed lifecycle hooks.

OBSERVABILITY.md Symptom 1 / 3 fix sections rewritten to describe
the new wiring.

Verified:
- make test → 37 passed
- make check → ruff/format/pyrefly clean
- manage.py check (no DJANGO_SETTINGS_MODULE) → no issues
- Process probe: runserver argv → TracerProvider; check/celery
  argv → ProxyTracerProvider (no-op)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
models.py top-level has no `tasks` dependency, so tasks.py can import
`ImageTask` (and `Image`) at top level without re-introducing a
circular import. Drops the in-function `from .models import ImageTask`
and the TYPE_CHECKING block for `Image`.

Net: one runtime lazy import remains in the models/tasks pair
(`models.py:create_and_dispatch` → `generate_thumbnail`, intentional
and commented). Satisfies task 07 acceptance.

Verified: make test → 37 passed; make check → clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors the gallery query-count assertion: 5 images + tasks, then
asserts the API list endpoint stays at 4 queries (session + user +
images + prefetched tasks). Closes the last task 08 gap; bulk of the
test work landed earlier in commit 9ac9b94.

Test count 37 → 38; make check still clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- task 10: automated gates green (make test 38 passed, make check
  clean); runtime/docker gates noted as user-driven.
- task 05: Debug Toolbar manual check marked superseded by task 08's
  programmatic django_assert_num_queries assertions.
- task 00_overview: new "Outcome (2026-05-16)" section with the LOC
  delta vs merge-base d6ac5ce — production code -72 (-4.5%),
  tests +137 (task 08). Deferred items listed with rationale.
- CLAUDE.md §0: updated to "Refinement closing" phase with LOC
  outcome and pointer to 00_overview.md.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- POSTGRES_DB: django_thumbnail_test → django_thumbnail. The previous
  value sat unused — Django connects to the maintenance DB and
  creates test_django_thumbnail itself. Matching settings/base.py
  defaults removes a reviewer's "wait, which DB?" pause.
- Drop redundant DJANGO_SETTINGS_MODULE env on the Test step.
  pyproject.toml [tool.pytest.ini_options] already pins it.
- Add "Dump postgres logs on failure" step mirroring the smoke
  workflow — gives diagnostic context if the DB connection breaks.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Refinement plan complete — drop tasks/refinement checklists and the
related CLAUDE.md sections (current-state header, tasks/ tree entry,
session-start and per-task rules).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Disable redundant uv@jamie-bitflight-skills (python-engineering:uv covers it).
- Remove 6 ghost rows from .claude/skills/README.md (onboard, ticket, pr-review,
  pr-summary, worktree-commit-merge, htmx-patterns); celery-patterns kept for H1.
- Update SKILLS_HIRE.md to record F1/F2 completion.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace pyright/ty references with pyrefly across hooks, agents, skills,
and commands to match actual dev dependency (pyrefly>=1.0.0) and CI usage
(make check runs python -m pyrefly check). Six locations updated:
.claude/settings.json hook, settings.md, code-reviewer agent,
systematic-debugging skill, code-quality skill, and fix command.

Mark P1 done in SKILLS_HIRE.md and record F3/F4/F5 type-checker portions
resolved; remaining work in F3/F4/F5 is htmx-related, gated on P2.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Decide P2: never adopt htmx (no package installed, frontend uses plain JS
fetch + JsonResponse). Apply consequences:

- F4 systematic-debugging: drop Django Debug Toolbar and HTMX sections
- F5 code-reviewer agent: drop HTMX checklist, partial-return branch,
  and htmx-alpine-patterns integration line
- F6 django-forms/templates/pytest-django-patterns: insert
  "htmx not used in this project" note above htmx sections; preserve
  content for potential future adoption
- F3 code-quality: remove stray HTMX HX-Request checklist row

Add H1 celery-patterns skill grounded in actual project code
(images/tasks.py, images/models.py, django_thumbnail/celery.py,
django_thumbnail/settings/test.py): @shared_task convention,
idempotency rule, exponential backoff retry, pass-IDs pattern,
Image/ImageTask state flow with real field names, and
CELERY_TASK_ALWAYS_EAGER testing mode.

Mark wave 1 complete in SKILLS_HIRE.md.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…, policy

Add three new local skills and a plugin enablement policy. Conclude
all remaining SKILLS_HIRE.md tasks (H2, H3, P3, P4, C1).

New skills (English, grounded in actual project code):

- storage-s3 — boto3 singleton construction (internal_s3 / public_s3 in
  images/storage.py), MinIO endpoint split (in-cluster vs public-host),
  presign patterns, signature_version="s3v4", smoke-test verification
  via head_object, and patching-by-import-path guidance.

- opentelemetry-patterns — span.set_attributes(dict) batch rule,
  web vs Celery worker telemetry separation (apps.CoreConfig.ready
  vs worker_process_init signal), SimpleSpanProcessor over BSP for
  fork safety (rationale from commit af308f6), per-process
  OTEL_SERVICE_NAME override in docker-compose, OTEL_SDK_DISABLED
  in settings/test.py.

Policy:

- .claude/SKILLS_POLICY.md catalogues the 36 python-engineering
  plugin skills: 12 approved (concept-only references), 24 disabled
  (CLI/TUI, packaging, orchestrator family, ty/prek-assumptions,
  stack-incompatible). Maps each project need to its local tool.
  Defines the quality-check chain (P4): /fix inner-loop,
  /code-quality + code-reviewer pre-PR, make check + make test in CI.

P3 decision: django-extensions stays uninstalled. No usage signals
(zero references in pyproject.toml, settings, Makefile, agents,
commands, or user memory). The skill remains as a future-activation
reference with a DORMANT banner and explicit activation steps.
Also fix the leftover example "core.EmailAccount / metabox.Thread"
to use the project's actual models.

Post-audit fixes (factual accuracy):

- storage-s3: rename cited Image methods to real names
  (upload_original, delete_from_storage) per images/models.py:86,89
- opentelemetry-patterns: document per-process OTEL_SERVICE_NAME
  override (web=django-thumbnail-app, worker=django-thumbnail-worker)
  required by Jaeger trace separation in test_smoke.py
- SKILLS_POLICY: correct plugin skill count from 38 to 36

Convert F6 htmx notes and django-extensions dormant banner from
Traditional Chinese to English to match the rest of the skill corpus.

SKILLS_HIRE.md fully closed; all three waves are now ✅ DONE.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
All planned tasks (P1-P4, F1-F6, H1-H3, C1) are complete. Remove the
internal planning docs and strip the dead "Companion docs" footer from
SKILLS_POLICY.md. Operational policy lives in:

- .claude/SKILLS_POLICY.md (plugin enablement + quality-check chain)
- .claude/skills/*/SKILL.md (concrete skills)
- .claude/skills/django-extensions/SKILL.md (DORMANT banner)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Enable flake8-annotations (ANN) in ruff with ANN401 active so Any is
rejected. Add per-file ignores for tests/migrations/manage.py/conftest.

Annotate 32 previously untyped sites:
- decorators: PEP 695 ParamSpec on login_required_json to preserve
  wrapped view signatures end-to-end
- management commands: handle(*args: object, **options: object) -> None;
  s3 cached_property typed as S3Client (TYPE_CHECKING)
- storage: 3 __init__ return None
- tasks: generate_thumbnail self: Task (Celery bind=True)
- views: 5 HTML views -> HttpResponse
- models: _fmt_ts(dt: datetime | None)
Drop per-file ignores for manage.py, conftest.py, and tests/. Keep
migrations ignored — Django regenerates them via makemigrations and
would overwrite annotations.

- manage.py: main() -> None
- conftest.py: fixtures typed with Client, User, tuple[Client, User]
- factories.py: post_generation password method fully typed
- tests/: 38 test methods get -> None; fixtures typed via
  DjangoAssertNumQueries and Iterator[None] for yielding mock_s3
TonyPythoneer and others added 10 commits May 16, 2026 07:32
Collapse 6 duplicated test methods into 3 parametrized ones using
@pytest.mark.parametrize with named ids:

- test_login: matrix over (password, expected_status)
- test_create (images): matrix over (payload, expected_status)
- test_delete (images): matrix over (owned, expected_status, image_remains)
- test_create_task: matrix over (payload_kind, expected_status, task_created)
- test_get_task: matrix over (owned, expected_status)

18 test methods → 12 methods producing 17 cases. Tests with distinct
assertion shapes (cancel_task, list_query_count_constant, unauthenticated)
stay separate. ids=[...] keeps failure messages readable.
factory-boy class calls return the factory class to type checkers,
breaking IDE attribute resolution on returned model fields. Wrap
each factory in a helper that asserts the model return type via
typing.cast, restoring IDE/pyrefly visibility on .id, .latest_task,
refresh_from_db, etc. Pyrefly errors across tests drop from 40 to 8.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Promote generate_thumbnail max_retries to a module-level constant so
  tests reference the int directly instead of celery's untyped Task attr.
- Wrap celery .apply call in a Callable cast (stubs misreport it as list).
- factories.UserFactory.password: cast self to User (factory-boy injects
  the instance at runtime; pyrefly enforces LSP on self annotations).
- test_models: null-check latest_task() before .id access.
- pyrefly.toml: drop tests/ and management/ excludes; add manage.py.

Result: 0 pyrefly errors across the full project (was 40 with tests
included), no new type:ignore lines.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DELETE /api/tasks/<id>/ let any owner revoke a running Celery task.
Task lifecycle should be controlled internally (admin/management), not
exposed to end users — removing the verb closes that surface.

tasks_detail now accepts GET only; AsyncResult import and the
test_cancel_task case are dropped.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- test_tasks: split test_status_outcomes into test_direct_call (success
  and invalid_image) and test_resilient_to_download_failure (retry
  outcomes). Each method is now straight-line with no branching; the
  parametrize matrices align with their actual call mode.
- test_views_html: hoist URL names into class-level *_viewname
  attributes, matching the convention already used in test_api.py.
- test_smoke: rename infra-check helpers so each name announces the
  stack layer it covers (async worker, storage, observability) and
  drop the redundant docstrings. Add Jaeger query retry loop so the
  trace assertion no longer relies on a single fixed sleep.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wire a TracerProvider + OTLP gRPC exporter and RequestsInstrumentor in
the smoke test process so outbound HTTP calls inject traceparent and
emit client spans. Web/worker spans become children under a single
trace, enabling end-to-end visibility in Jaeger.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Move Jaeger HTTP polling, response schemas, and helpers into a single
test-only module (images/tests/jaeger.py) with a JaegerClient singleton
mirroring the storage.py pattern. Response shapes are typed via
TypedDict, with a shared KeyedValue covering process/span/log fields
and NotRequired warnings to reflect Jaeger's optional payloads.

Switch smoke process to SimpleSpanProcessor so spans export
synchronously on end, removing the need for force_flush() before the
events assertion. The smoke root span now runs inline in the test
method via tracer.start_as_current_span, with trace_id read from the
span context. The trace is verified in two passes: service presence
inside the active span, then events on the root span after it ends.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Extract hardcoded test1/test2 credentials from create_test_users
command and smoke test into shared settings._test_users module.
Imported only by local.py/test.py; base.py defaults to empty list
so prod inherits no preloaded accounts. Command no-ops with warning
when TEST_USERS is empty (prod safety guard).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolves pyrefly untyped-import warning on requests usage in
smoke tests (images/tests/test_smoke.py).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@TonyPythoneer
TonyPythoneer deleted the test/user-account branch May 17, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant